[orion-server]: auto targets generation for buck2#1312
Conversation
Signed-off-by: Xiaoyang Han <lux1an@qq.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Signed-off-by: Han Xiaoyang <lux1an@qq.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds automatic target generation for buck2 builds by downloading BUCK and .buckconfig files from a blob API and executing buck2 commands to determine build targets dynamically.
- Introduces a new buck2 module with functionality to download build configuration files and execute buck2 commands
- Modifies the build request API to accept hash values for BUCK and .buckconfig files instead of hardcoded targets
- Adds retry logic for file downloads and proper error handling throughout the buck2 workflow
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| orion-server/src/main.rs | Adds buck2 module import |
| orion-server/src/buck2.rs | New module implementing buck2 file download and target generation functionality |
| orion-server/src/api.rs | Updates BuildRequest struct and task_handler to use dynamic target generation |
| orion-server/Cargo.toml | Adds dependencies for tokio-retry and reqwest |
| orion-server/.env | Adds MONOBASE_URL environment variable |
Comments suppressed due to low confidence (1)
| let last_line = get_buck2_targets_last_line(&tmp_dir_path)?; | ||
|
|
||
| // Clean up the temporary directory after getting the result | ||
| //TODO: Cleanup should happen in a finally block or using RAII to ensure temporary files are removed even if buck2 command fails. Consider using a guard or defer pattern. TempDirGuard , for example. |
There was a problem hiding this comment.
The TODO comment identifies a legitimate issue. The current cleanup logic can fail to execute if the buck2 command panics or if there's an early return. Consider implementing a proper RAII guard using a struct with Drop trait or using the tempfile crate's TempDir.
| pub struct BuildRequest { | ||
| repo: String, | ||
| target: String, | ||
| buck_hash: String, |
There was a problem hiding this comment.
[nitpick] The field name 'buck_hash' is ambiguous. Consider renaming to 'buck_file_hash' or 'buck_build_file_hash' to clearly indicate this is the hash for the BUCK build file.
| buck_hash: String, | |
| #[serde(rename = "buck_hash")] | |
| buck_file_hash: String, |
| // Download and get buck2 targets first | ||
| let target = match download_and_get_buck2_targets(&req.buck_hash, &req.buckconfig_hash).await { | ||
| Ok(target) => target, | ||
| Err(e) => { | ||
| tracing::error!("Failed to download buck2 targets: {}", e); | ||
| return ( | ||
| StatusCode::INTERNAL_SERVER_ERROR, | ||
| Json(serde_json::json!({ "message": format!("Failed to download buck2 targets: {}", e) })), | ||
| ).into_response(); | ||
| } |
There was a problem hiding this comment.
The buck2 target generation happens synchronously in the request handler, which could cause request timeouts for large projects. Consider moving this to an async background task or implementing a timeout mechanism.
| // Download and get buck2 targets first | |
| let target = match download_and_get_buck2_targets(&req.buck_hash, &req.buckconfig_hash).await { | |
| Ok(target) => target, | |
| Err(e) => { | |
| tracing::error!("Failed to download buck2 targets: {}", e); | |
| return ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(serde_json::json!({ "message": format!("Failed to download buck2 targets: {}", e) })), | |
| ).into_response(); | |
| } | |
| // Download and get buck2 targets first, with a timeout | |
| let buck2_timeout = Duration::from_secs(30); | |
| let target = match timeout( | |
| buck2_timeout, | |
| download_and_get_buck2_targets(&req.buck_hash, &req.buckconfig_hash) | |
| ).await { | |
| Ok(Ok(target)) => target, | |
| Ok(Err(e)) => { | |
| tracing::error!("Failed to download buck2 targets: {}", e); | |
| return ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(serde_json::json!({ "message": format!("Failed to download buck2 targets: {}", e) })), | |
| ).into_response(); | |
| } | |
| Err(_) => { | |
| tracing::error!("Timeout while downloading buck2 targets"); | |
| return ( | |
| StatusCode::REQUEST_TIMEOUT, | |
| Json(serde_json::json!({ "message": "Timeout while downloading buck2 targets" })), | |
| ).into_response(); | |
| } |
No description provided.